Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check Service and Resource name maximum length #81

Merged
merged 7 commits into from
Feb 10, 2025

Conversation

adcreare
Copy link
Contributor

@adcreare adcreare commented Jan 20, 2025

Closes #80
I went with some stricter limits on service names and s3 buckets with some exceptions for current cases we have. Open to alternative ideas.

Additionally: Had to make unrelated changes to a couple of tests as what was in main was no longer passing tests - coverage test will have to be temporary removed for merging

  • node change changed the output of assert and that caused some tests to fail on exact match - added regex to make this more reliable
  • Validate test failed as checkdigit/typescript-config has changed its setting for skipLibCheck a few times, hard set this value now

@adcreare adcreare added the MINOR label Jan 20, 2025
@adcreare adcreare self-assigned this Jan 20, 2025
Copy link
Contributor

@le-cong le-cong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm


const MAXIMUM_SERVICE_NAME_LENGTH = 20;
const SERVICE_NAME_LENGTH_EXCEPTIONS = new Set([
'current-certification',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to env variable or parameter

@adcreare
Copy link
Contributor Author

adcreare commented Feb 6, 2025

Converted to environment variables, ready for another look

@adcreare adcreare requested a review from carlansley February 6, 2025 20:48
Copy link

github-actions bot commented Feb 6, 2025

✅ PR review status - All reviews completed and approved!

@adcreare adcreare merged commit f75f023 into main Feb 10, 2025
5 of 6 checks passed
@adcreare adcreare deleted the test-service-resource-length branch February 10, 2025 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expand publish beta to check resource and service name lengths
3 participants